Fix finding the correct cplusplus compiler #23146

bingos · 2025-03-21T14:05:35Z

ExtUtils::CBuilder was using a slightly maverick method for finding the matching cplusplus compiler to the c compiler used to build perl.

On a Linux system with a perl built with the Oracle Developer cc cc='/opt/oracle/developerstudio12.6/bin/cc'

Errors were observed:
"c++: error: unrecognized command line option ‘-KPIC’; did you mean ‘-fPIC’?"

The cplusplus command for Oracle Developer suite is CC not c++ and the detection was picking up the system c++ (g++).

If there is a ccpath, the code should exhaust all the options and not fail through to using no path.

Clang/Clang++ were missing from the C/C++ mappings so I have added those as well.

ExtUtils::CBuilder was using a slightly maverick method for finding the matching cplusplus compiler to the c compiler used to build perl. On a Linux system with a perl built with the Oracle Developer cc cc='/opt/oracle/developerstudio12.6/bin/cc' Errors were observed: "c++: error: unrecognized command line option ‘-KPIC’; did you mean ‘-fPIC’?" The cplusplus command for Oracle Developer suite is CC not c++ and the detection was picking up the system c++ (g++). If there is a ccpath, the code should exhaust all the options and not fail through to using no path.

jkeenan · 2025-03-25T00:34:53Z

Are there any changes that could be added to CBuilder's test suite that would exercise these code changes?

haarg · 2025-03-25T06:31:28Z

dist/ExtUtils-CBuilder/lib/ExtUtils/CBuilder/Base.pm

+          if( can_run( $cxx1 ) ) {
+              $self->{config}{cxx} = $cxx1;
+	            last;
+          }


The indentation here doesn't match the rest of the file, and has added literal tabs mixed with the spaces.

I didn't actually change that from the original, the literal tab was there before. But let me fix that.

bingos · 2025-03-25T11:24:35Z

The test t/03-cpluscplus.t was the test that highlighted to me that the detection behaviour was wrong.

mauke · 2025-04-18T03:18:36Z

The following patch would test and detect the bad detection logic on common platforms, I believe:

--- dist/ExtUtils-CBuilder/t/03-cplusplus.t
+++ dist/ExtUtils-CBuilder/t/03-cplusplus.t
@@ -26,7 +26,18 @@ else {
   plan tests => 7;
 }
 
-ok $b, "created EU::CB object";
+{
+    # GH #23146
+    my $fake_cc = File::Spec->rel2abs(File::Spec->catfile(qw(some directory what doesnt exist), 'cc'));
+    my $b = ExtUtils::CBuilder->new(
+        quiet => $quiet,
+        config => {
+            cc => $fake_cc,
+        },
+    );
+
+    is $b->{config}{cxx}, $fake_cc, "did not search PATH for C++ compiler when given absolute path to C compiler";
+}
 
 ok $b->have_cplusplus, "have_cplusplus";

(I removed the ok $b test because it is pointless: At this point in the file, we've already called $b->have_cplusplus, so if $b wasn't defined, we would have error'd out earlier.)

See: #23146 (comment)

jkeenan · 2025-04-19T19:31:44Z

The following patch would test and detect the bad detection logic on common platforms, I believe:

--- dist/ExtUtils-CBuilder/t/03-cplusplus.t
+++ dist/ExtUtils-CBuilder/t/03-cplusplus.t
@@ -26,7 +26,18 @@ else {
   plan tests => 7;
 }
 
-ok $b, "created EU::CB object";
+{
+    # GH #23146
+    my $fake_cc = File::Spec->rel2abs(File::Spec->catfile(qw(some directory what doesnt exist), 'cc'));
+    my $b = ExtUtils::CBuilder->new(
+        quiet => $quiet,
+        config => {
+            cc => $fake_cc,
+        },
+    );
+
+    is $b->{config}{cxx}, $fake_cc, "did not search PATH for C++ compiler when given absolute path to C compiler";
+}
 
 ok $b->have_cplusplus, "have_cplusplus";

(I removed the ok $b test because it is pointless: At this point in the file, we've already called $b->have_cplusplus, so if $b wasn't defined, we would have error'd out earlier.)

I checked-out this p.r. locally, applied @mauke's patch, and hacked up CBuilder/Base.pm with some debugging statements. (See: https://github.com/Perl/perl5/tree/jkeenan/pr23146-with-mauke-patch-20250419.) On that basis, I believe @mauke's patch would do what he claims it would; hence, it should be applied to the p.r. and the p.r. to blead -- unless @bingos objects.

jkeenan

Please add @mauke's patch to test file; then okay to merge.

ap · 2025-04-24T16:55:56Z

@bingos Speaking for the PSC, we’d like to ship this. Can you get to it?

bingos · 2025-04-24T17:13:09Z

I have no objections.

bingos · 2025-04-24T19:41:43Z

Added the test should be good to rebase and merge once the checks are done.

jkeenan · 2025-04-25T00:38:09Z

This p.r. is repeatedly failing one test file on 2 Windows runs in our GH CI. See, e.g.,
https://github.com/Perl/perl5/actions/runs/14650246952/job/41114622775?pr=23146. Excerpt of failure output (with some line re-breaks for readability):

../dist/ExtUtils-CBuilder/t/02-link.t ................................ skipped: link_executable() is not implemented yet on MSWin32
cl : Command line warning D9035 : option 'o' has been deprecated and 
will be removed in a future release
error building t\cplust.dll from "t\cplust.obj" at 
../../lib/ExtUtils/CBuilder/Platform/Windows.pm line 241.
# Looks like your test exited with 25 just after 6.
../dist/ExtUtils-CBuilder/t/03-cplusplus.t ........................... 
Dubious, test returned 25 (wstat 6400, 0x1900)
Failed 1/7 subtests 
...
Test Summary Report
-------------------
../dist/ExtUtils-CBuilder/t/03-cplusplus.t                         (Wstat: 6400 (exited 25) Tests: 6 Failed: 0)
  Non-zero exit status: 25
  Parse errors: Bad plan.  You planned 7 tests but ran 6.
../dist/Net-Ping/t/450_service.t                                   (Wstat: 0 Tests: 26 Failed: 0)
  TODO passed:   9
../ext/IPC-Open3/t/IPC-Open3.t                                     (Wstat: 0 Tests: 53 Failed: 0)
  TODO passed:   33
Files=2914, Tests=1203522, 2829 wallclock secs (149.69 usr +  5.75 sys = 155.44 CPU)
Result: FAIL

We can use someone with access to Windows to debug this.

bingos · 2025-04-26T12:47:02Z

I've messed up the tests, I'll fix that. I have a Windows Server 2019 virtual host with Visual Studio Enterprise 2019 installed so I am running a build/test of the branch on there.

bingos · 2025-04-26T13:59:19Z

I am not entirely sure why but the test added to exercise ccpath is the cause of the problems on Windows with MSVC. The test uses 'cc' as the compiler which makes EUCB believe that it is now using GCC rather than MSVC and sets all the flags for compiling and linking to GCC

If I move that test to the end everything is good.

bingos · 2025-04-26T15:32:17Z

Merged as ea0856c

bingos added 2 commits March 21, 2025 14:00

Added clang/clang++ to the C->C++ compiler mappings

5cda3c1

bingos requested a review from Leont March 22, 2025 20:07

haarg reviewed Mar 25, 2025

View reviewed changes

Remove literal tabs

a9cee21

mauke approved these changes Apr 18, 2025

View reviewed changes

jkeenan added a commit that referenced this pull request Apr 19, 2025

Test patch proposed for GH #23146

407b414

See: #23146 (comment)

jkeenan self-requested a review April 19, 2025 19:40

jkeenan requested changes Apr 19, 2025

View reviewed changes

Leont approved these changes Apr 24, 2025

View reviewed changes

Added mauke's test

02915d0

Make the fakecc test more distinct

65f36ba

Fix the test plan

258985d

Move ccpath test to the end

b6519cc

bingos closed this Apr 26, 2025

bingos deleted the bingos/cbuilder_cplusplus branch April 26, 2025 15:35

Leont mentioned this pull request May 13, 2025

ExtUtils-CBuilder/t/03-cplusplus.t failed when ENV{CXX} is defined #23277

Closed

lillianokok mentioned this pull request Jun 5, 2025

executing the test command dist/ExtUtils-CBuilder/t/03-cplusplusplus fails, #23355

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix finding the correct cplusplus compiler #23146

Fix finding the correct cplusplus compiler #23146

Uh oh!

bingos commented Mar 21, 2025 •

edited

Loading

Uh oh!

jkeenan commented Mar 25, 2025

Uh oh!

haarg Mar 25, 2025

Uh oh!

bingos Mar 25, 2025

Uh oh!

bingos commented Mar 25, 2025

Uh oh!

mauke commented Apr 18, 2025

Uh oh!

jkeenan commented Apr 19, 2025

Uh oh!

jkeenan left a comment

Uh oh!

ap commented Apr 24, 2025

Uh oh!

bingos commented Apr 24, 2025

Uh oh!

bingos commented Apr 24, 2025

Uh oh!

jkeenan commented Apr 25, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

Uh oh!

Fix finding the correct cplusplus compiler #23146

Fix finding the correct cplusplus compiler #23146

Uh oh!

Conversation

bingos commented Mar 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkeenan commented Mar 25, 2025

Uh oh!

haarg Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

bingos Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

bingos commented Mar 25, 2025

Uh oh!

mauke commented Apr 18, 2025

Uh oh!

jkeenan commented Apr 19, 2025

Uh oh!

jkeenan left a comment

Choose a reason for hiding this comment

Uh oh!

ap commented Apr 24, 2025

Uh oh!

bingos commented Apr 24, 2025

Uh oh!

bingos commented Apr 24, 2025

Uh oh!

jkeenan commented Apr 25, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

bingos commented Apr 26, 2025

Uh oh!

Uh oh!

bingos commented Mar 21, 2025 •

edited

Loading